Add allowed list check on EII implementations attributes by chenyukang · Pull Request #159173 · rust-lang/rust

chenyukang · 2026-07-12T08:23:19Z

View all comments

Fixes #158293
Fixes #159015

r? @bjorn3

rustbot · 2026-07-12T08:23:22Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-07-12T21:38:57Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

mejrs · 2026-07-12T22:11:43Z

Maybe we ought to have an allowlist of attributes rather than error on these particular attributes? I'm also a bit worried about someone later down the line adding some new (possibly link related) attribute and not including it in this check.

chenyukang · 2026-07-12T23:53:26Z

Maybe we ought to have an allowlist of attributes rather than error on these particular attributes? I'm also a bit worried about someone later down the line adding some new (possibly link related) attribute and not including it in this check.

maybe allowlist contains lint levels, cfg, doc, inline, cold, optimize, coverage、sanitize, must_use, deprecated?

bjorn3 · 2026-07-14T10:03:28Z

Seems like a reasonable list.

chenyukang · 2026-07-14T10:17:45Z

Seems like a reasonable list.

I tried to get a more overwhole status on eii, this #125418 is the best place to track.
seems the current code is different with original rfc's syntax now.
so we are moving on the implementation and then draft another rfc in future?

bjorn3 · 2026-07-14T10:24:10Z

I don't know if the current EII syntax is expected to be stabilized eventually or if the intent is for the syntax from the RFC to be implemented eventually with #[eii] just being a temporary thing while we work on the internal implementation.

mejrs · 2026-07-14T10:51:21Z

For reference, attributes on eii declarations are handled via allowlist:

rust/compiler/rustc_builtin_macros/src/eii.rs

Lines 180 to 228 in da80ed0

    
           fn split_attrs( 
        
               ecx: &mut ExtCtxt<'_>, 
        
               span: Span, 
        
               attrs: ThinVec<Attribute>, 
        
           ) -> (ThinVec<Attribute>, ThinVec<Attribute>, ThinVec<Attribute>) { 
        
               let mut macro_attributes = ThinVec::new(); 
        
               let mut foreign_item_attributes = ThinVec::new(); 
        
               let mut default_attributes = ThinVec::new(); 
        
               for attr in attrs { 
        
                   match attr.name() { 
        
                       // Inline only matters for the default function being inlined into callsites 
        
                       Some(sym::inline) => default_attributes.push(attr), 
        
                       // If an eii is marked a lang item, that's because we want to call its declaration, so 
        
                       // mark the foreign item as the lang item 
        
                       Some(sym::lang) => foreign_item_attributes.push(attr), 
        
                       // Deprecating an eii means deprecating the macro and the foreign item 
        
                       Some(sym::deprecated) => { 
        
                           foreign_item_attributes.push(attr.clone()); 
        
                           macro_attributes.push(attr); 
        
                       } 
        
                       // The stability of an EII affects the usage of the macro and calling the foreign item 
        
                       Some(sym::stable) | Some(sym::unstable) => { 
        
                           foreign_item_attributes.push(attr.clone()); 
        
                           macro_attributes.push(attr); 
        
                       } 
        
                       // `#[track_caller]` goes on the foreign item only: it's the symbol callers link 
        
                       // against, so it must carry the flag for call sites to pass the caller location. 
        
                       // Implementations derive it during codegen (see `EiiImpls` in `codegen_attrs.rs`), 
        
                       // so it must not be routed onto the default impl here. 
        
                       Some(sym::track_caller) => { 
        
                           foreign_item_attributes.push(attr); 
        
                       } 
        
                       // Doc attributes should be forwarded to the macro and the foreign item, since those are 
        
                       // the two items you interact with as a user. 
        
                       // FIXME: idk yet how EIIs show up in docs, might want to customize 
        
                       _ if attr.is_doc_comment() => { 
        
                           foreign_item_attributes.push(attr.clone()); 
        
                           macro_attributes.push(attr); 
        
                       } 
        
                       Some(sym::eii) => unreachable!("should already be filtered out"), 
        
                       _ => { 
        
                           ecx.dcx().emit_err(EiiAttributeNotSupported { span, attr_span: attr.span() }); 
        
                       } 
        
                   } 
        
               } 
        
               (macro_attributes, foreign_item_attributes, default_attributes) 
        
           }

I would expect attributes on implementations to be handled similarly.

seems the current code is different with original rfc's syntax now.
so we are moving on the implementation and then draft another rfc in future?

It's an experiment so we can diverge from the original proposal. Once we are happy with the implementation the rfc should be updated accordingly.

chenyukang · 2026-07-14T15:28:07Z

    }
+
+    // Check EII implementation attributes with a allowed list
+    fn check_eii_impl_attrs(&self, attrs: &[Attribute], eii_impls: &[EiiImpl]) {


I moved the checking to here in ast_validation, since macro expansion finished,
and leaves the final attributes, similar with
https://github.com/chenyukang/rust/blob/30cc6b46e8bb20318b05443cb80f1e50119ba971/compiler/rustc_ast_passes/src/ast_validation.rs#L522-L532

the old place where depends on HIR, HIR converts some attributes into variants without a uniform path or span.

View changes since the review

I think the current location is the best we can currently do.
Integrating this into the attribute parsing system somehow would be ideal, but I don't see a way to do this. Doing this after attribute parsing (like in the HIR) is not desirable since the spans are gone then.

bjorn3 · 2026-07-16T13:12:21Z

I think someone more familiar with attribute parsing/validation should review this. I don't know the advantages and disadvantages of the various locations attributes are validated.

mejrs · 2026-07-16T14:43:07Z

Can you update a test to include #[naked] as well?

I think someone more familiar with attribute parsing/validation should review this. I don't know the advantages and disadvantages of the various locations attributes are validated.

I think it's ~fine to do it like this. We could have the eii impl macro leave behind a trace attribute so we can still do this check post-expansion and keep it in check_attr.rs. Not sure what the best course of action is. If a trace attribute is useful elsewhere, then maybe we should do that.

r? @JonathanBrouwer

rustbot · 2026-07-16T14:43:11Z

JonathanBrouwer is currently at their maximum review capacity.
They may take a while to respond.

JonathanBrouwer · 2026-07-16T15:42:02Z

(might take me a few days to get around to it, but happy to take a look)

JonathanBrouwer

r=me after solving conflicts and including #[naked] as mejrs asked

View changes since this review

mejrs · 2026-07-20T13:30:20Z

+            sym::cfg_trace,
+            sym::cfg_attr_trace,


Do these two still do anything after #159266?

View changes since the review

no, we need to remove them.

rustbot · 2026-07-20T13:35:35Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

chenyukang · 2026-07-23T09:29:24Z

@bors r=JonathanBrouwer

rust-bors · 2026-07-23T09:29:26Z

📌 Commit 4e53ac1 has been approved by JonathanBrouwer

It is now in the queue for this repository.

@bjorn3

…ngle-eii, r=JonathanBrouwer Add allowed list check on EII implementations attributes Fixes rust-lang#158293 Fixes rust-lang#159015 r? @bjorn3

Rollup of 18 pull requests Successful merges: - #138618 (Support using const pointers in asm `const` operand) - #150161 (Remove 'static requirement on try_as_dyn) - #158362 (trait solver: account for universes from replace_bound_vars) - #159173 (Add allowed list check on EII implementations attributes) - #159466 (cmse: clear variant-dependent padding in `enum`s) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #157905 (Update comments and add tests for `-Zrandomize-layout` for some guaranteed ZSTs) - #159041 (Reorganize `tests/ui/issues` [22/N]) - #159108 (Reorganize `tests/ui/issues` [23/N]) - #159138 (doc: document wasm import symbol mangling) - #159531 (Reorganize `tests/ui/issues` [28/N]) - #159608 (early_otherwise: Don't hoist dereferences when the otherwise branch is reachable) - #159612 (Reorganize `tests/ui/issues` [29/N]) - #159653 (run `tests/assembly-llvm/asm/aarch64-types.rs` for `aarch64_be`) - #159759 (rustc-dev-guide subtree update) - #159761 (Remove outdated comment for resolve_vars_with_obligations)

@bjorn3

…ngle-eii, r=JonathanBrouwer Add allowed list check on EII implementations attributes Fixes rust-lang#158293 Fixes rust-lang#159015 r? @bjorn3

@bjorn3

…ngle-eii, r=JonathanBrouwer Add allowed list check on EII implementations attributes Fixes rust-lang#158293 Fixes rust-lang#159015 r? @bjorn3

Rollup of 19 pull requests Successful merges: - #150161 (Remove 'static requirement on try_as_dyn) - #158362 (trait solver: account for universes from replace_bound_vars) - #159173 (Add allowed list check on EII implementations attributes) - #159466 (cmse: clear variant-dependent padding in `enum`s) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #157905 (Update comments and add tests for `-Zrandomize-layout` for some guaranteed ZSTs) - #158766 (Promote riscv64-unknown-linux-musl to tier 2 with host tools) - #159041 (Reorganize `tests/ui/issues` [22/N]) - #159108 (Reorganize `tests/ui/issues` [23/N]) - #159138 (doc: document wasm import symbol mangling) - #159531 (Reorganize `tests/ui/issues` [28/N]) - #159608 (early_otherwise: Don't hoist dereferences when the otherwise branch is reachable) - #159612 (Reorganize `tests/ui/issues` [29/N]) - #159653 (run `tests/assembly-llvm/asm/aarch64-types.rs` for `aarch64_be`) - #159667 (Make some parser structured suggestions verbose and tweak their wording) - #159759 (rustc-dev-guide subtree update) - #159761 (Remove outdated comment for resolve_vars_with_obligations)

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 12, 2026

rustbot assigned bjorn3 Jul 12, 2026

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from 23be887 to 9c36383 Compare July 12, 2026 08:25

This comment has been minimized.

Sign in to view

bjorn3 reviewed Jul 12, 2026

View reviewed changes

Comment thread compiler/rustc_passes/src/check_attr.rs Outdated

bjorn3 added the F-extern_item_impls `#![feature(extern_item_impls)]` label Jul 12, 2026

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from 9c36383 to 81a1b49 Compare July 12, 2026 21:38

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from 81a1b49 to c7cb405 Compare July 12, 2026 21:41

This comment has been minimized.

Sign in to view

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from c7cb405 to 8e3fe6d Compare July 12, 2026 23:43

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jul 12, 2026

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch 2 times, most recently from 35bc796 to 30cc6b4 Compare July 14, 2026 15:24

chenyukang commented Jul 14, 2026

View reviewed changes

chenyukang changed the title ~~Reject no_mangle on EII implementations~~ Add allowed list check on EII implementations attributes Jul 14, 2026

This comment has been minimized.

Sign in to view

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from 30cc6b4 to 265407a Compare July 15, 2026 00:10

This comment has been minimized.

Sign in to view

mejrs mentioned this pull request Jul 16, 2026

error on naked EII implementations #159382

Closed

cezarbbb mentioned this pull request Jul 16, 2026

Naked EII impl functions are currently not working #158293

Open

rustbot assigned JonathanBrouwer and unassigned bjorn3 Jul 16, 2026

JonathanBrouwer approved these changes Jul 20, 2026

View reviewed changes

JonathanBrouwer added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2026

mejrs reviewed Jul 20, 2026

View reviewed changes

Add allow_list checking for eii implementation attributes

4e53ac1

chenyukang force-pushed the yukang-fix-159015-no-mangle-eii branch from 265407a to 4e53ac1 Compare July 20, 2026 13:35

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 23, 2026

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 18 pull requests #159762

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 19 pull requests #159766

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 19 pull requests #159769

Open

Uh oh!

Uh oh!

Conversation

chenyukang commented Jul 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 12, 2026

Uh oh!

This comment has been minimized.

Uh oh!

rustbot commented Jul 12, 2026

Uh oh!

This comment has been minimized.

mejrs commented Jul 12, 2026

Uh oh!

chenyukang commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjorn3 commented Jul 14, 2026

Uh oh!

chenyukang commented Jul 14, 2026

Uh oh!

bjorn3 commented Jul 14, 2026

Uh oh!

mejrs commented Jul 14, 2026

Uh oh!

chenyukang Jul 14, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bjorn3 commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejrs commented Jul 16, 2026

Uh oh!

rustbot commented Jul 16, 2026

Uh oh!

JonathanBrouwer commented Jul 16, 2026

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 20, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenyukang Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 20, 2026

Uh oh!

chenyukang commented Jul 23, 2026

Uh oh!

rust-bors Bot commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chenyukang commented Jul 12, 2026 •

edited by rustbot

Loading

chenyukang commented Jul 12, 2026 •

edited

Loading

chenyukang Jul 14, 2026 •

edited by rustbot

Loading

bjorn3 commented Jul 16, 2026 •

edited

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading

mejrs Jul 20, 2026 •

edited by rustbot

Loading